chore: separate storage and account cursors#229

Merged

emhane merged 4 commits intounstablefrom

meyer9/split-storage-account-cursor

Oct 15, 2025

Collaborator

meyer9 commented Oct 14, 2025 •

edited

Loading

This PR separates storage and account cursors which makes sense because these two cursors may have very different seek/next implementations. In the case of in-memory, a single cursor impl can still handle both, but for MDBX, it makes sense to implement separate cursors.

Closes #236


          chore: separate storage and account cursors

2bb5a36

Copilot AI review requested due to automatic review settings

October 14, 2025 20:40

meyer9 requested review from dhyaniarun1993, emhane, itschaindev and sadiq1971 as code owners

October 14, 2025 20:40

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Pull Request Overview

This PR separates the storage and account cursors into distinct APIs to allow for specialized implementations in different storage backends like MDBX while maintaining compatibility with in-memory implementations.

Replaces single trie_cursor method with separate account_trie_cursor and storage_trie_cursor methods
Updates trait definitions to include separate cursor types for storage and account tries
Migrates all test code to use the new cursor-specific methods

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/optimism/trie/src/api.rs	Defines separate cursor types and methods for storage/account tries
crates/optimism/trie/src/in_memory.rs	Implements new cursor methods for in-memory storage
crates/optimism/trie/src/db/store.rs	Updates MDBX implementation with separate cursor methods
crates/optimism/trie/tests/in_memory.rs	Updates all test calls to use specific cursor methods
crates/optimism/trie/src/backfill.rs	Updates backfill operations to use new cursor methods

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

meyer9 mentioned this pull request

chore: add deletion tests for op proofs storage #230

Merged

dhyaniarun1993 approved these changes

View reviewed changes

emhane approved these changes

View reviewed changes

crates/optimism/trie/src/in_memory.rs Show resolved Hide resolved

crates/optimism/trie/src/db/store.rs Show resolved Hide resolved

emhane commented Oct 15, 2025

also pr description is missing which issue it references/closes

sadiq1971 approved these changes

View reviewed changes


          Merge branch 'unstable' into meyer9/split-storage-account-cursor

53f693b

Collaborator Author

meyer9 commented Oct 15, 2025 •

edited

Loading

~~fix for this upstream: paradigmxyz#19043~~

Actually decided to just import optimism primitives here with serde-bincode-compat.


          fix: optimism primitives should include serde-bincode-compat

b918de4

meyer9 requested review from BioMark3r and theochap as code owners

October 15, 2025 16:44

wiz-b4c72f16a4 bot commented Oct 15, 2025 •

edited

Loading

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	-
Sensitive Data	-

Total	-

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.


          chore: add explanation to weird import

6f3d862

emhane approved these changes

View reviewed changes

crates/optimism/exex/Cargo.toml

Comment on lines +22 to +24

+              # for ethereum types, `serde-bincode-compat` is added by `reth-storage-api`, however this does not work with `op` until
+              # `reth-storage-api` is updated to support `op`, so we add it here.
+              reth-optimism-primitives = { workspace = true, features = ["reth-codec", "serde-bincode-compat", "serde"] }

emhane Oct 15, 2025

hopefully this is solved in #242 so that we don't have to mess with the manifests outside of the optimism directory for this op feature

emhane added this pull request to the merge queue

Merged via the queue into unstable with commit 040a33b

40 checks passed

emhane deleted the meyer9/split-storage-account-cursor branch

October 15, 2025 17:47

github-merge-queue bot pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

f098118

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane mentioned this pull request

feat(deps): Add serde-bincode-compat feature to reth-trie #250

Merged

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

d552c79

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

3313a87

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

b6f2830

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

12c1940

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

77b4e00

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

d767cdc

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

269b2ba

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

a069749

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

4cde270

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

71ad89c

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

b8604ed

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

3ef85f5

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

0c96bea

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

7e359d4

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

f079dfb

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

87ff6b1

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

b6c16dd

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

6616d61

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane added a commit to paradigmxyz/reth that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

e971aa4

Ref op-rs#241

- Fixes comment from deps fix in
op-rs#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

6316bad

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

088fe46

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

89db137

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

e1c9467

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

b03d9f2

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

f003b3f

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane added a commit to ethereum-optimism/optimism that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

15dfcdd

Ref op-rs/op-reth#241

- Fixes comment from deps fix in
op-rs/op-reth#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request


          chore: separate storage and account cursors (op-rs/op-reth#229)

3f4ceb9

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes op-rs/op-reth#236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request


          chore: add deletion tests for op proofs storage (op-rs/op-reth#230)

efdaa8a

Based on op-rs/op-reth#229

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes op-rs/op-reth#238

theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (op-rs/…

515f50f

…op-reth#250)

Ref op-rs/op-reth#241

- Fixes comment from deps fix in
op-rs/op-reth#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request


          chore: separate storage and account cursors (op-rs/op-reth#229)

125b542

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes op-rs/op-reth#236

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>

theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request


          chore: add deletion tests for op proofs storage (op-rs/op-reth#230)

b6ad32a

Based on op-rs/op-reth#229

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes op-rs/op-reth#238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

sadiq1971 sadiq1971 approved these changes

dhyaniarun1993 dhyaniarun1993 approved these changes

itschaindev Awaiting requested review from itschaindev itschaindev is a code owner

theochap Awaiting requested review from theochap theochap is a code owner

BioMark3r Awaiting requested review from BioMark3r

+1 more reviewer

emhane emhane approved these changes

Labels

None yet